Skip to content

update to catch Throwable for execute method #702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft

update to catch Throwable for execute method #702

wants to merge 2 commits into from

Conversation

@kaibocai
Copy link
Member

@kaibocai kaibocai commented Feb 14, 2023 •
edited
Loading

Issue describing the changes in this PR

resolves #295

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

Additional information

  • Need to test out on production, especially for durable functions.

Catching Exception will igore Error happend in java worker process or customer code. This resulted in host hanging up for 30mins and time out, since host never get chance to know that an Error happened in java worker. This is because Error won't be caught and it will break the current logics to response the host request through grpc.
image

Update to catch Throwable so that any Error type will also be caught and inform host through grpc.
Didn't re-throw the Throwable as there is not much difference, as long as we get this Error info passed back to host.

@kaibocai kaibocai marked this pull request as draft February 14, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anirudhgarg anirudhgarg Awaiting requested review from anirudhgarg

@TsuyoshiUshio TsuyoshiUshio Awaiting requested review from TsuyoshiUshio TsuyoshiUshio is a code owner

@amamounelsayed amamounelsayed Awaiting requested review from amamounelsayed

@shreyas-gopalakrishna shreyas-gopalakrishna Awaiting requested review from shreyas-gopalakrishna

@kamperiadis kamperiadis Awaiting requested review from kamperiadis

@vrdmr vrdmr Awaiting requested review from vrdmr vrdmr will be requested when the pull request is marked ready for review vrdmr is a code owner

@gavin-aguiar gavin-aguiar Awaiting requested review from gavin-aguiar gavin-aguiar will be requested when the pull request is marked ready for review gavin-aguiar is a code owner

@hallvictoria hallvictoria Awaiting requested review from hallvictoria hallvictoria will be requested when the pull request is marked ready for review hallvictoria is a code owner

@ahmedmuhsin ahmedmuhsin Awaiting requested review from ahmedmuhsin ahmedmuhsin will be requested when the pull request is marked ready for review ahmedmuhsin is a code owner

@swapnil-nagar swapnil-nagar Awaiting requested review from swapnil-nagar swapnil-nagar will be requested when the pull request is marked ready for review swapnil-nagar is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Exceptions in static blocks blocks not caught/recorded

2 participants

@kaibocai